refactor: change RawContentKey to alloy_primites::Bytes - #1431
Conversation
morph-dev
left a comment
There was a problem hiding this comment.
This is a good start, but I think that this PR should also make some changes in OverlayContentKey.
Here are the changes that we should do:
- remove
Into<Vec<u8>>from supertraits set (we should useto_bytesdirectly) - change
TryFrom<Vec<u8>, Error = ContentKeyError>toTryFrom<RawContentKey, Error = ContentKeyError>in supertraits set to_bytesfunction should returnRawContentKey
I think some of these are trivial, but some might require refactoring (I don't know until I try). If refactoring is not huge, I would put it into this PR, otherwise, it would be fine to split it into multiple PRs.
|
Also, see https://www.conventionalcommits.org/en/v1.0.0/ for failing check. |
a5d5ff6 to
feaf9d5
Compare
feaf9d5 to
046a4e1
Compare
|
Implemented |
morph-dev
left a comment
There was a problem hiding this comment.
Few comments, repeating in many places:
- we should use
RawContentKey::_instead ofBytes::_(example: Bytes::new())- this allows it easier to later replace
RawContentKeywith something else (instead of being alias) if we want to - it also makes code more explicit
- this allows it easier to later replace
- we should avoid unnecessary creating new objects, examples:
- use
RawContentKey::from_strdirectly (instead of usinghex_decodeto getVec<u8>and then convert that into RawContentKey) - remove
.clone()when not needed).
- use
morph-dev
left a comment
There was a problem hiding this comment.
Looks good! If you can address last 2 comments, and I will merge the PR.
| } | ||
|
|
||
| impl From<&StateContentKey> for Vec<u8> { | ||
| impl From<&StateContentKey> for RawContentKey { |
There was a problem hiding this comment.
This should be removed (similar to other ones)
|
You can ignore failing commitlint check, as I will squash and merge changes anyway. But for the future, problem is the commit message: "fix: CI fixes and resolved conflicts with master", because it starts with capital letter (maybe something that we should change in our configuration). |
585bbc8 to
88f0e87
Compare
|
Fixed the commit message and remove From impl for StateContentKey |
What was wrong?
Vec<u8>was being used to represent Raw Content Key.See #1404 for details
How was it #fixed?
Replaced it with
alloy_primites::BytesTo-Do